Fix phpstan/phpstan#6830: Variable inside loop might not be defined.#5213
Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Open
Fix phpstan/phpstan#6830: Variable inside loop might not be defined.#5213phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Conversation
- Modified intersectConditionalExpressions() in MutatingScope to match conditional expressions by their condition part when exact key match fails, then merge result type holders via and() - Added conditionExpressionTypeHoldersEqual() helper method - New regression test in tests/PHPStan/Rules/Variables/data/bug-6830b.php - Root cause: conditional expression keys include result types, which change across loop iterations when variables are reassigned, causing the intersection to drop them entirely
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When a variable is conditionally assigned before a
foreachloop and then used inside the loop under the same condition, PHPStan incorrectly reports "Variable $x might not be defined." This fix preserves conditional expression knowledge across loop iterations even when the variable's type evolves due to reassignment.Changes
intersectConditionalExpressions()insrc/Analyser/MutatingScope.phpto fall back to condition-based matching when exact key matching fails, merging result type holders viaand()conditionExpressionTypeHoldersEqual()private helper method for comparing condition parts of conditional expressionstests/PHPStan/Rules/Variables/data/bug-6830b.phpand corresponding test method intests/PHPStan/Rules/Variables/DefinedVariableRuleTest.phpRoot cause
intersectConditionalExpressions()matched conditional expression holders by their full key, which includes both the condition (e.g.,$do=true) and the result type+certainty (e.g.,int(9999) (Yes)). During foreach loop iterations, whenmergeWith()is called to combine the loop body scope with the original scope, the result type of a variable can change due to reassignment in the loop body (e.g., from9999to123). This caused the keys to differ, dropping the conditional expression entirely. On subsequent iterations, PHPStan no longer knew the variable was defined when the condition held.The fix adds a fallback: when no exact key match is found, it searches for a conditional expression with matching conditions (same guard expressions and types). If found, it merges the result type holders using
ExpressionTypeHolder::and(), which unions the types and ANDs the certainties. The exact key match is kept as a fast path for the common case.Test
Added
testBug6830binDefinedVariableRuleTestwith a test case that assigns a variable conditionally before a foreach loop, then uses and reassigns it inside the loop under the same condition. The test expects no errors (empty expected errors array).Fixes phpstan/phpstan#6830